Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Event listener interface and presence changes #183

Merged
merged 11 commits into from
Jan 26, 2024

Conversation

parfeon
Copy link
Contributor

@parfeon parfeon commented Jan 19, 2024

feat(listener): change the real-time event handling interface

feat(presence-state): state maintained after set state is used

user_id state for specified channels will be maintained by the SDK. State with subscribe calls has been improved.

feat(api): adding first-class citizens to access subscription

Adding Channel, ChannelGroup, ChannelMetadata and UuidMetadata entities to be first-class citizens to access APIs related to them. Currently, access is provided only for subscription APIs.

feat(auto-retry): added ability to exclude endpoints from retry

Added ability to configure request retry policies to exclude specific endpoints from retry.

feat(presence-state): state maintained after set state is used

`user_id` state for specified channels will be maintained by the SDK. State with subscribe calls
has been improved.

feat(api): adding first-class citizens to access subscription

Adding `Channel`, `ChannelGroup`, `ChannelMetadata` and `UuidMetadata` entities to be first-class
citizens to access APIs related to them. Currently, access is provided only for subscription APIs.

feat(auto-retry): added ability to exclude endpoints from retry

Added ability to configure request retry policies to exclude specific endpoints from retry.
@parfeon parfeon added priority: medium This PR should be reviewed after all high priority PRs. status: in progress This issue is being worked on. type: feature This PR contains new feature. labels Jan 19, 2024
@parfeon parfeon self-assigned this Jan 19, 2024
@parfeon
Copy link
Contributor Author

parfeon commented Jan 19, 2024

Adding PR for initial review purpose (interface shouldn't change) and documentation. There are still troubles with contract tests.

Copy link
Contributor

@Xavrax Xavrax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job!
First iteration of review here.
Some questions and nitpicks.

Cargo.toml Outdated Show resolved Hide resolved
examples/presence_state.rs Show resolved Hide resolved
examples/subscribe.rs Outdated Show resolved Hide resolved
src/core/channel.rs Outdated Show resolved Hide resolved
src/core/entity.rs Outdated Show resolved Hide resolved
src/dx/presence/builders/heartbeat.rs Outdated Show resolved Hide resolved
src/dx/presence/builders/set_presence_state.rs Outdated Show resolved Hide resolved
src/dx/presence/event_engine/effects/mod.rs Show resolved Hide resolved
src/dx/subscribe/subscription.rs Show resolved Hide resolved
src/dx/subscribe/subscription_set.rs Outdated Show resolved Hide resolved
examples/subscribe.rs Outdated Show resolved Hide resolved
@parfeon parfeon marked this pull request as ready for review January 24, 2024 23:03
Added information about how `Subscription` and `SubscriptionSet` instance leaving the scope can
affect subscription loop.

refactor(presence): leave from all channels trigger different event

Made changes which will trigger different event when unsubscribing from all channels and groups.
Copy link
Contributor

@seba-aln seba-aln left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that you addressed all the comments from @Xavrax . So...
image

Make sure that `drop()` won't trigger `unregister` for `Subscription` and `SubscriptionSet` clones
(which has been created with `Clone::clone()`).
Add new type to make it clear which parameters should be passed to `client.subscription(...)`.
Copy link
Contributor

@seba-aln seba-aln left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still LGTM

@parfeon
Copy link
Contributor Author

parfeon commented Jan 25, 2024

@pubnub-release-bot release

@parfeon parfeon merged commit c3b921f into master Jan 26, 2024
9 checks passed
@parfeon parfeon deleted the CLEN-1696/refactor/accumulate-state-for-heartbeat branch January 26, 2024 06:22
@pubnub-release-bot
Copy link

🚀 Release successfully completed 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: medium This PR should be reviewed after all high priority PRs. status: in progress This issue is being worked on. type: feature This PR contains new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants